chore: refactor pull request template for better structure and UI guidance#6200
chore: refactor pull request template for better structure and UI guidance#6200s3arthak wants to merge 3 commits intoLibreSign:mainfrom
Conversation
…dance Signed-off-by: Sarthak Kharka <[email protected]> :wq
|
@s3arthak could you check de DCO issue? https://github.com/LibreSign/libresign/pull/6200/checks?check_run_id=58120799765 |
s3arthak
force-pushed
the
chore/refactor-pr-template
branch
from
c62b179 to
2762962
Compare
vitormattos
left a comment
vitormattos
left a comment
There was a problem hiding this comment.
Sounds that you generated this using an AI and the AI mixed the details with the template. Check again the code. Look at the original template at row 32 have a details tag with info about how to test into GitHub Codespaces, it's important but not necessary to be displayed for everyone. Fix this and after I will make a new review.
…nventions Signed-off-by: Sarthak Kharka <[email protected]>
s3arthak
force-pushed
the
chore/refactor-pr-template
branch
from
3835889 to
990302f
Compare
|
@s3arthak look the comments about the tag details. |
…idance Signed-off-by: Sarthak Kharka <[email protected]>
|
|
||
| <!-- | ||
| Please check the type of change your pull request introduces. Remove all that is unrelated and remove the comment block too, maintaining only the type of your PR: | ||
| <details> |
There was a problem hiding this comment.
It would be helpful to move this block to the end of the document for better organization, as per the original template.
I noticed that the AI prompt removed many code details and made several changes. Could you clarify the reasoning behind these adjustments?
|
|
||
| ## 🎨 UI (Frontend) Changes | ||
|
|
||
| > Complete this section only if this pull request includes UI changes. |
There was a problem hiding this comment.
The example template contains a code snippet within comments, such as:
<!--
░░░░░░░░░░░░░░░░░░░░░
░█████░░██████░█████░
░░███░░░░███░░░███░░
░░███ ░░░░███░░░███░░
░░███░░░░███░░░███░░
░░░█████████░░░█████░
░░░░░░░░░░░░░░░░░░░░
Feel free to remove this section when your PR only affects The backend/API code.
-->
This way, anyone opening a new PR will be notified that if it's only about the frontend, they should remove the backend section; otherwise, if it's only about the backend, they should remove the frontend section.
|
|
||
| 🏚️ Before | 🏡 After | ||
| -- | -- | ||
| Screenshot before | Screenshot after |
There was a problem hiding this comment.
At the example, also have instructions about light and dark theme inside a comment that would be good to have here.
| Screenshot before | Screenshot after | |
| Screenshot before | Screenshot after | |
| <!-- ☀️ Light theme | 🌑 Dark Theme --> |
| - [ ] UI changes implemented | ||
| - [ ] Visual consistency checked | ||
| - [ ] Accessibility considerations applied (if applicable) |
There was a problem hiding this comment.
The idea is to the developer create the tasks herself describing what's made and what's pending. Maybe with ellipsis could give this idea, or maybe also would be good to have a visible hidden comment here too explaining that, like - [ ] <!-- your tasks here -->
| - [ ] UI changes implemented | |
| - [ ] Visual consistency checked | |
| - [ ] Accessibility considerations applied (if applicable) | |
| - [ ] ... |
| - [ ] Tested on Firefox | ||
| - [ ] UI does not rely on browser-specific behavior | ||
| - [ ] Design reviewed, approved, or inspired by existing LibreSign / Nextcloud UI | ||
| - [ ] User-facing documentation updated (if required) |
There was a problem hiding this comment.
Maybe would be good to have a reference to user documentation of LibreSign website: https://docs.libresign.coop/user_manual/ and repository of website: https://github.com/LibreSign/documentation/ together with this item
|
|
||
| ## 🛠️ API / Backend Changes | ||
|
|
||
| > Complete this section only if this pull request includes backend or API changes. |
There was a problem hiding this comment.
The comment about frontend that I made before.
| - [ ] Backend logic implemented or updated | ||
| - [ ] API contracts reviewed | ||
| - [ ] Database or migration changes documented (if applicable) |
There was a problem hiding this comment.
The same as previous comment about tasks
|
|
||
| ### 🏁 API Checklist | ||
| - [ ] Unit and/or integration tests added, or not required | ||
| - [ ] API documentation in `docs/` updated (if required) |
There was a problem hiding this comment.
The API documentation for LibreSign is not found in the docs folder. Instead, it is generated using the Composer command: composer openapi, which updates the openapi.json file. It would be beneficial to include a link to the developer documentation and to create an issue in the LibreSign documentation repository outlining how to use composer openapi. This issue should offer specific instructions tailored to LibreSign, since the Nextcloud developer documentation already covers the general process of generating OpenAPI documentation: Nextcloud Developer Manual.
| - [ ] API documentation in `docs/` updated (if required) | |
| - [ ] API documentation updated with the command `composer openapi` if necessary |
| --- | ||
|
|
||
| ## 📋 General Checklist | ||
| - [ ] PR is focused on a single concern | ||
| - [ ] Code follows project conventions | ||
| - [ ] Relevant tests added or justified | ||
| - [ ] Documentation updated if needed | ||
| - [ ] No breaking changes (or clearly documented) | ||
|
|
||
| --- | ||
|
|
||
| ## ℹ️ Additional Notes | ||
| Add any additional context if necessary. No newline at end of file |
There was a problem hiding this comment.
It seems like there is too much information required when creating a PR.
| --- | |
| ## 📋 General Checklist | |
| - [ ] PR is focused on a single concern | |
| - [ ] Code follows project conventions | |
| - [ ] Relevant tests added or justified | |
| - [ ] Documentation updated if needed | |
| - [ ] No breaking changes (or clearly documented) | |
| --- | |
| ## ℹ️ Additional Notes | |
| Add any additional context if necessary. |
|
@s3arthak, why did you close the PR? Did you need more help? |
|
I closed the PR because I realized the changes were more structural than intended for a template file, and I didn’t want to create extra review overhead. I’d be happy to contribute to other issues that better match my current skills. |
- Clearly separate UI (frontend) and API (backend) sections - Add before/after screenshot table for UI changes - Include checklists for tasks, testing, documentation, accessibility, etc. - Preserve Codespaces testing instructions in a collapsible block - Add visual hints (ASCII art comments) to guide removal of irrelevant sections - Update API documentation instruction to use `composer openapi` - Add optional items: browser testing, accessibility, design review, capabilities - Include AI disclosure checkbox for transparency This addresses the requirements of issue LibreSign#5534 and incorporates feedback from the previous attempt (LibreSign#6200). Fixes LibreSign#5534 Signed-off-by: Guilherme Carvalho <[email protected]>
- Clearly separate UI (frontend) and API (backend) sections - Add before/after screenshot table for UI changes - Include checklists for tasks, testing, documentation, accessibility, etc. - Preserve Codespaces testing instructions in a collapsible block - Add visual hints (ASCII art comments) to guide removal of irrelevant sections - Update API documentation instruction to use `composer openapi` - Add optional items: browser testing, accessibility, design review, capabilities - Include AI disclosure checkbox for transparency This addresses the requirements of issue #5534 and incorporates feedback from the previous attempt (#6200). Fixes #5534 Signed-off-by: Guilherme Carvalho <[email protected]>
- Clearly separate UI (frontend) and API (backend) sections - Add before/after screenshot table for UI changes - Include checklists for tasks, testing, documentation, accessibility, etc. - Preserve Codespaces testing instructions in a collapsible block - Add visual hints (ASCII art comments) to guide removal of irrelevant sections - Update API documentation instruction to use `composer openapi` - Add optional items: browser testing, accessibility, design review, capabilities - Include AI disclosure checkbox for transparency This addresses the requirements of issue #5534 and incorporates feedback from the previous attempt (#6200). Fixes #5534 Signed-off-by: Guilherme Carvalho <[email protected]>
2 participants
Fixes #5534
📝 Summary
This pull request refactors the pull request template to improve structure, readability, and contributor guidance.
It clearly separates frontend (UI) and backend (API) sections, retains the Codespaces testing instructions, and adds a dedicated UI checklist with before/after screenshots.
🧪 How to Test (GitHub Codespaces)
No runtime changes were made.
Please verify that the new pull request template renders correctly when opening a new pull request and that all sections (UI, API, Codespaces) are clearly structured.
🎨 UI (Frontend) Changes
Not applicable. This change only affects repository documentation/templates.
🛠️ API / Backend Changes
Not applicable. This change does not modify backend or API behavior.
📋 Checklist